Skip to content

Use volatile access for CoreCLR dictionary layout publication#129763

Merged
davidwrighton merged 4 commits into
mainfrom
copilot/fix-crash-in-methodtable
Jun 24, 2026
Merged

Use volatile access for CoreCLR dictionary layout publication#129763
davidwrighton merged 4 commits into
mainfrom
copilot/fix-crash-in-methodtable

Conversation

Copilot AI commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

This change addresses a rare CoreCLR crash in generic dictionary/method resolution caused by racing publication of dictionary layout pointers. The fix makes both publication and observation of those pointers use volatile semantics in the affected EEClass and InstantiatedMethodDesc paths.

  • Dictionary layout publication

    • Switch EEClass::SetDictionaryLayout and InstantiatedMethodDesc::IMD_SetDictionaryLayout to VolatileStore.
    • This makes layout expansion visible to concurrent readers with the expected release semantics.
  • Dictionary layout reads

    • Switch EEClass::GetDictionaryLayout, InstantiatedMethodDesc::IMD_GetDictionaryLayout, and InstantiatedMethodDesc::GetDictLayoutRaw to VolatileLoad.
    • This removes the remaining raw read path for m_pDictLayout, so all readers participate in the same publication protocol.
  • Scope

    • The change is intentionally narrow: only the layout pointer accessors were updated.
    • No behavior outside dictionary layout publication/consumption is changed.
return HasOptionalFields() ? VolatileLoad(&GetOptionalFields()->m_pDictLayout) : NULL;

// ...

VolatileStore(&pIMD->m_pDictLayout, pNewLayout);

Copilot AI self-assigned this Jun 23, 2026
Copilot AI review requested due to automatic review settings June 23, 2026 18:36
Copilot AI removed the request for review from Copilot June 23, 2026 18:37
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @agocke
See info in area-owners.md if you want to be subscribed.

Co-authored-by: davidwrighton <10779849+davidwrighton@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 23, 2026 19:02
Co-authored-by: davidwrighton <10779849+davidwrighton@users.noreply.github.com>
Copilot AI requested review from Copilot and removed request for Copilot June 23, 2026 19:15
Copilot AI changed the title [WIP] Fix crash in MethodTable::MethodDataObject::FillEntryDataForAncestor Use volatile access for CoreCLR dictionary layout publication Jun 23, 2026
Copilot AI requested a review from davidwrighton June 23, 2026 19:15
@davidwrighton davidwrighton marked this pull request as ready for review June 23, 2026 21:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens the publication/observation protocol for generic dictionary layout pointers in CoreCLR by switching the affected EEClass and InstantiatedMethodDesc layout-pointer accessors to use volatile semantics, reducing the chance of readers observing a partially-published update under concurrency.

Changes:

  • Update InstantiatedMethodDesc dictionary-layout reads to use VolatileLoad, including the previously raw read path (GetDictLayoutRaw).
  • Update InstantiatedMethodDesc dictionary-layout writes to use VolatileStore.
  • Update EEClass dictionary-layout reads/writes (via optional fields) to use VolatileLoad/VolatileStore.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/coreclr/vm/method.hpp Uses VolatileLoad/VolatileStore for InstantiatedMethodDesc::m_pDictLayout access across getters/setters, including the “raw” getter.
src/coreclr/vm/class.h Uses VolatileLoad/VolatileStore for EEClassOptionalFields::m_pDictLayout access in EEClass accessors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants